Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

update dependencies and tools #87

Merged
merged 11 commits into from
Jan 11, 2023
Merged

update dependencies and tools #87

merged 11 commits into from
Jan 11, 2023

Conversation

bcressey
Copy link
Contributor

Issue number:
N/A

Description of changes:
Update dependencies and tools.

Testing done:
Built Bottlerocket variants for x86_64 and aarch64.

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@bcressey
update base image to Fedora 37
0a880d7 
Signed-off-by: Ben Cressey <bcressey@amazon.com>
@bcressey
update Buildroot to 2022.11
54db7df 
Signed-off-by: Ben Cressey <bcressey@amazon.com>
@bcressey
switch to GCC 11
9a34807 
Signed-off-by: Ben Cressey <bcressey@amazon.com>
@bcressey
update LLVM to 15.0.6
fc9b1e6 
Signed-off-by: Ben Cressey <bcressey@amazon.com>
stmcginnis
stmcginnis approved these changes Jan 10, 2023
View reviewed changes
Copy link
Contributor

@stmcginnis stmcginnis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unable to test myself, but based on described testing and looking over the changes, everything looks fine to me. I guess I'd give it some bonus confidence points based on the author too.

jpmcb
jpmcb approved these changes Jan 10, 2023
View reviewed changes
Copy link
Contributor

@jpmcb jpmcb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Built locally via make on both x86 and arm hosts 👍🏼

@jpmcb jpmcb mentioned this pull request Jan 10, 2023
Closed
jpculp
jpculp reviewed Jan 10, 2023
View reviewed changes
Copy link
Member

@jpculp jpculp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we bump tokio in the license-tool?

@yeazelm
Copy link
Contributor

yeazelm commented Jan 10, 2023

We should make sure to grab Rust 1.66.1 which will include a fix for a CVE in cargo CVE-2022-46176

@yeazelm yeazelm marked this pull request as draft January 10, 2023 20:50
@yeazelm yeazelm marked this pull request as ready for review January 10, 2023 20:50
webern
webern approved these changes Jan 10, 2023
View reviewed changes
Dockerfile Outdated Show resolved Hide resolved
@bcressey
update Rust to 1.66.1
767ed59 
Signed-off-by: Ben Cressey <bcressey@amazon.com>
@bcressey
update SPDX license data to 3.19
eee45dd 
Signed-off-by: Ben Cressey <bcressey@amazon.com>
@bcressey
update cargo-deny to 0.13.5
4d2b88c 
Signed-off-by: Ben Cressey <bcressey@amazon.com>
@bcressey bcressey mentioned this pull request Jan 11, 2023
Merged
@bcressey
license-tool: update dependencies
5d04e73 
Signed-off-by: Ben Cressey <bcressey@amazon.com>
@bcressey
license-scan: update dependencies
e2e8b68 
Signed-off-by: Ben Cressey <bcressey@amazon.com>
@bcressey
refactor package install stages
0bfb362 
Instead of the full Development Tools group, install the packages we
actually need to build the various toolchains and other host tools.

Move the packages that are only needed to build Bottlerocket to the
later stage that's intended for that purpose.

Signed-off-by: Ben Cressey <bcressey@amazon.com>
@bcressey
bump version to 0.29.0
8074a18 
Signed-off-by: Ben Cressey <bcressey@amazon.com>
@bcressey
Copy link
Contributor Author

I've started running into this error, which looks like awslabs/aws-c-common#964:

> [sdk-final 34/35] RUN   mkdir .netscape &&   certutil -N --empty-password:
#192 2.713 Fatal error condition occurred in /home/builder/aws-sdk-cpp-src/crt/aws-crt-cpp/crt/aws-c-common/source/allocator.c:209: allocator != ((void *)0)
#192 2.713 Exiting Application
#192 2.713 No call stack information available

I'm going to revert the aws-sdk-cpp update in order to keep this moving forward. I tried going back to the previous version in this PR - 1.10.22 - but it's also failing now.

zmrow
zmrow approved these changes Jan 11, 2023
View reviewed changes
Copy link
Contributor

@zmrow zmrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🍾

arnaldo2792
arnaldo2792 approved these changes Jan 11, 2023
View reviewed changes
Dockerfile
Comment on lines 41 to 44
dnf config-manager --set-disabled \
fedora-modular updates-modular fedora-cisco-openh264 && \
dnf clean all && \
fedora-modular \
updates-modular \
fedora-cisco-openh264 \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: these commands are reformatted here, but were added in the first commit of the series. Probably the diff will be smaller if the format doesn't change(?).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they should have been this newer format originally, and the sooner we change it then the sooner it's in better shape for ongoing updates where the diff can show individual lines more clearly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, so probably keep this format in the first commit of the series

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This gets back to whether we treat our unit of change as individual commits or the pull request. I personally am not as concerned about individual commits in a PR unless there is a chance one of those commits may need to be reverted at some point.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it would be worth the hassle.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point - I'd tidy this up if there were another change, but I don't think it's worth the retesting burden just for commit history. Style-wise, that was consistent with the rest of Dockerfile at the time the change was made, so it's not indefensible. 😀

@bcressey bcressey merged commit beef290 into bottlerocket-os:develop Jan 11, 2023
@bcressey bcressey deleted the tool-updates branch January 11, 2023 19:01
@yeazelm yeazelm mentioned this pull request Mar 9, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stmcginnis stmcginnis stmcginnis approved these changes

@jpculp jpculp jpculp approved these changes

@arnaldo2792 arnaldo2792 arnaldo2792 approved these changes

@zmrow zmrow zmrow approved these changes

@webern webern webern approved these changes

@jpmcb jpmcb jpmcb approved these changes

@markusboehme markusboehme Awaiting requested review from markusboehme

@foersleo foersleo Awaiting requested review from foersleo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@bcressey @yeazelm @zmrow @stmcginnis @jpculp @webern @arnaldo2792 @jpmcb